Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

process: add externalMemory to process.memoryUsage #9587

Closed
wants to merge 1 commit into from
Closed

process: add externalMemory to process.memoryUsage #9587

wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Nov 12, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

process

Description of change

Well, title says it all.

cc @nodejs/collaborators

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 12, 2016
@indutny
Copy link
Member Author

indutny commented Nov 12, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/4833/

@indutny indutny added process Issues and PRs related to the process subsystem. lts-watch-v4.x semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 12, 2016
@mscdex
Copy link
Contributor

mscdex commented Nov 12, 2016

Can we shorten it to just 'external'?

@indutny
Copy link
Member Author

indutny commented Nov 12, 2016

Mmm... ok :)

@indutny indutny force-pushed the feature/memory-usage-external branch from d5349b9 to f2f5efc Compare November 12, 2016 23:49
@indutny
Copy link
Member Author

indutny commented Nov 12, 2016

Fixed.

mcollina
mcollina approved these changes Nov 13, 2016
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

targos
targos approved these changes Nov 13, 2016
View reviewed changes
doc/api/process.md
}
```

`heapTotal` and `heapUsed` refer to V8's memory usage.
`external` refer to the memory usage of C++ objects bound to JavaScript
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: refers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@indutny indutny force-pushed the feature/memory-usage-external branch from f2f5efc to 2172c06 Compare November 13, 2016 09:31
@indutny
Copy link
Member Author

indutny commented Nov 13, 2016

Thank you for review! Will land it tomorrow, unless anyone will object.

@jbergstroem
Copy link
Member

@indutny just a heads up on commit title length: https://ci.nodejs.org/job/node-test-commitmsg/27/tapTestReport/

silverwind
silverwind approved these changes Nov 13, 2016
View reviewed changes
Copy link
Contributor

@silverwind silverwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, suggesting renaming commit to

process: add process.memoryUsage.external

addaleax
addaleax approved these changes Nov 13, 2016
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@indutny indutny force-pushed the feature/memory-usage-external branch from 2172c06 to e35f0e2 Compare November 13, 2016 20:46
@indutny
Copy link
Member Author

indutny commented Nov 13, 2016

Landed in 5a61b99, thank you everyone!

@indutny indutny closed this Nov 13, 2016
@indutny indutny deleted the feature/memory-usage-external branch November 13, 2016 20:49
indutny added a commit that referenced this pull request Nov 13, 2016
@indutny
process: add process.memoryUsage.external
5a61b99 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@bnoordhuis
Copy link
Member

@indutny Remember the 48/72 hour rule.

@indutny
Copy link
Member Author

indutny commented Nov 13, 2016

Gosh, 48? Not 24? Sorry!

addaleax pushed a commit that referenced this pull request Nov 22, 2016
@indutny @addaleax
process: add process.memoryUsage.external
163397a 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Fishrock123 Fishrock123 mentioned this pull request Nov 22, 2016
Merged
2 tasks
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
@Fishrock123
2016-11-22, Version 7.2.0 (Current)
70404e7 
Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
@Fishrock123
2016-11-22, Version 7.2.0 (Current)
c1aa949 
This is a security release impacting Windows 10 users.

Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
  - Fixed a potential buffer overflow when writing data to console on
Windows 10. (CVE-2016-9551)
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* tls: Fixed a memory leak when writes were queued on TLS connection
that was destroyed during handshake. (Fedor Indutny)
#9626
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
@Fishrock123
2016-11-22, Version 7.2.0 (Current)
db3f12f 
This is a security release impacting Windows 10 users.

Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
  - Fixed a potential buffer overflow when writing data to console on
Windows 10. (CVE-2016-9551)
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* tls: Fixed a memory leak when writes were queued on TLS connection
that was destroyed during handshake. (Fedor Indutny)
#9626
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
imyller added a commit to imyller/meta-nodejs that referenced this pull request Nov 28, 2016
@imyller
2016-11-22, Version 7.2.0 (Current)
bc886e7 
This is a security release impacting Windows 10 users.

    Notable changes:

    * crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
    `this`. (Kirill Fomichev) nodejs/node#9398
    * dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
    (Ben Noordhuis) nodejs/node#9296 &
    nodejs/node#9296
    * libuv: Upgrade to v1.10.1 (cjihrig)
    nodejs/node#9647
      - Fixed a potential buffer overflow when writing data to console on
    Windows 10. (CVE-2016-9551)
    * process: Added a new `external` property to the data returned by
    `memoryUsage()`. (Fedor Indutny)
    nodejs/node#9587
    * tls: Fixed a memory leak when writes were queued on TLS connection
    that was destroyed during handshake. (Fedor Indutny)
    nodejs/node#9626
    * V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
    nodejs/node#9697
    * v8: The data returned by `getHeapStatistics()` now includes three new
    fields: `malloced_memory`, `peak_malloced_memory`, and
    `does_zap_garbage`. (Gareth Ellis)
    nodejs/node#8610

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
@MylesBorins MylesBorins mentioned this pull request Jan 6, 2017
Closed
9 tasks
MylesBorins pushed a commit that referenced this pull request Jan 13, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
f16effe 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 13, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
2912da4 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
1226ce4 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
67abbb6 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This was referenced Jan 24, 2017
Merged
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
8ea4487 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
@indutny @MylesBorins
process: add process.memoryUsage.external
23a573f 
PR-URL: #9587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins added a commit that referenced this pull request Feb 21, 2017
@MylesBorins
2017-02-21, Version 4.8.0 'Argon' (LTS)
58ab54f 
Notable Changes:

* child_process: add shell option to spawn() (cjihrig)
  #4598
* crypto:
  * add ALPN Support (Shigeki Ohtsu)
    #2564
  * allow adding extra certs to well-known CAs (Sam Roberts)
    #9139
* deps:
  * v8: expose statistics about heap spaces (Ben Ripkens)
    #4463
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
  #5333
* process:
  * add `externalMemory` to `process` (Fedor Indutny)
    #9587
  * add process.cpuUsage() (Patrick Mueller)
    #10796
MylesBorins added a commit that referenced this pull request Feb 21, 2017
@MylesBorins
2017-02-21, Version 6.10.0 'Boron' (LTS)
1b6fd44 
Notable Changes:

The SEMVER-MINOR changes include:

*  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
   #9139
*  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
   #9234
*  process: add `process.memoryUsage.external` (Fedor Indutny)
   #9587
*  src: add wrapper for process.emitWarning() (Sam Roberts)
   #9139

Notable SEMVER-PATCH changes include:

*  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
   #10253
*  repl: allow autocompletion for scoped packages (Evan Lucas)
   #10296
MylesBorins added a commit that referenced this pull request Feb 22, 2017
@MylesBorins
2017-02-21, Version 4.8.0 'Argon' (LTS)
49c5ff4 
Notable Changes:

* child_process: add shell option to spawn() (cjihrig)
  #4598
* crypto:
  * add ALPN Support (Shigeki Ohtsu)
    #2564
  * allow adding extra certs to well-known CAs (Sam Roberts)
    #9139
* deps:
  * v8: expose statistics about heap spaces (Ben Ripkens)
    #4463
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
  #5333
* process:
  * add `externalMemory` to `process` (Fedor Indutny)
    #9587
  * add process.cpuUsage() (Patrick Mueller)
    #10796

PR-URL: #10973
MylesBorins added a commit that referenced this pull request Feb 22, 2017
@MylesBorins
2017-02-21, Version 6.10.0 'Boron' (LTS)
aecb085 
Notable Changes:

The SEMVER-MINOR changes include:

*  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
   #9139
*  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
   #9234
*  process: add `process.memoryUsage.external` (Fedor Indutny)
   #9587
*  src: add wrapper for process.emitWarning() (Sam Roberts)
   #9139

Notable SEMVER-PATCH changes include:

*  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
   #10253
*  repl: allow autocompletion for scoped packages (Evan Lucas)
   #10296

PR-URL: #10974
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 4.8.0 'Argon' (LTS)
bddd5cc 
    Notable Changes:

    * child_process: add shell option to spawn() (cjihrig)
      nodejs/node#4598
    * crypto:
      * add ALPN Support (Shigeki Ohtsu)
        nodejs/node#2564
      * allow adding extra certs to well-known CAs (Sam Roberts)
        nodejs/node#9139
    * deps:
      * v8: expose statistics about heap spaces (Ben Ripkens)
        nodejs/node#4463
    * fs: add the fs.mkdtemp() function. (Florian MARGAINE)
      nodejs/node#5333
    * process:
      * add `externalMemory` to `process` (Fedor Indutny)
        nodejs/node#9587
      * add process.cpuUsage() (Patrick Mueller)
        nodejs/node#10796

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 6.10.0 'Boron' (LTS)
edbc1bf 
    Notable Changes:

    The SEMVER-MINOR changes include:

    *  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
       nodejs/node#9139
    *  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
       nodejs/node#9234
    *  process: add `process.memoryUsage.external` (Fedor Indutny)
       nodejs/node#9587
    *  src: add wrapper for process.emitWarning() (Sam Roberts)
       nodejs/node#9139

    Notable SEMVER-PATCH changes include:

    *  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
       nodejs/node#10253
    *  repl: allow autocompletion for scoped packages (Evan Lucas)
       nodejs/node#10296

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 4.8.0 'Argon' (LTS)
2ee49f0 
    Notable Changes:

    * child_process: add shell option to spawn() (cjihrig)
      nodejs/node#4598
    * crypto:
      * add ALPN Support (Shigeki Ohtsu)
        nodejs/node#2564
      * allow adding extra certs to well-known CAs (Sam Roberts)
        nodejs/node#9139
    * deps:
      * v8: expose statistics about heap spaces (Ben Ripkens)
        nodejs/node#4463
    * fs: add the fs.mkdtemp() function. (Florian MARGAINE)
      nodejs/node#5333
    * process:
      * add `externalMemory` to `process` (Fedor Indutny)
        nodejs/node#9587
      * add process.cpuUsage() (Patrick Mueller)
        nodejs/node#10796

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 6.10.0 'Boron' (LTS)
f99e583 
    Notable Changes:

    The SEMVER-MINOR changes include:

    *  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
       nodejs/node#9139
    *  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
       nodejs/node#9234
    *  process: add `process.memoryUsage.external` (Fedor Indutny)
       nodejs/node#9587
    *  src: add wrapper for process.emitWarning() (Sam Roberts)
       nodejs/node#9139

    Notable SEMVER-PATCH changes include:

    *  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
       nodejs/node#10253
    *  repl: allow autocompletion for scoped packages (Evan Lucas)
       nodejs/node#10296

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@addaleax addaleax addaleax approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jbergstroem jbergstroem jbergstroem approved these changes

@silverwind silverwind silverwind approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.